-
Notifications
You must be signed in to change notification settings - Fork 17.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AP_DDS: Status Message #28337
base: master
Are you sure you want to change the base?
AP_DDS: Status Message #28337
Conversation
libraries/AP_DDS/AP_DDS_Client.cpp
Outdated
@@ -42,6 +43,7 @@ static constexpr uint16_t DELAY_GEO_POSE_TOPIC_MS = 33; | |||
static constexpr uint16_t DELAY_CLOCK_TOPIC_MS = 10; | |||
static constexpr uint16_t DELAY_GPS_GLOBAL_ORIGIN_TOPIC_MS = 1000; | |||
static constexpr uint16_t DELAY_PING_MS = 500; | |||
static constexpr uint16_t DELAY_STATUS_TOPIC_MS = 500; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this event driven instead rather than poll?
This introduces as most 500mS delay on known the vehicle has failsafed which may be a long time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there events I can use to drive this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are not. ArduPilot does not have any such architecture. The best approach is to poll the state quickly and only publish on change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set to 100ms
Clean commits before merge. |
libraries/AP_DDS/AP_DDS_Client.cpp
Outdated
msg.flying = AP_Notify::flags.flying; | ||
uint8_t fs_iter = 0; | ||
msg.failsafe_size = 0; | ||
if ( AP_Notify::flags.failsafe_radio ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: little whitespace within the brackets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whitespaces removed
libraries/AP_DDS/AP_DDS_Client.cpp
Outdated
{ | ||
// Fill the new message. | ||
msg.vehicle_type = static_cast<uint8_t>(AP::fwversion().vehicle_type); | ||
msg.armed = AP_Notify::flags.armed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be hal.util->get_soft_armed()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set as suggested
libraries/AP_DDS/AP_DDS_Client.cpp
Outdated
msg.vehicle_type = static_cast<uint8_t>(AP::fwversion().vehicle_type); | ||
msg.armed = AP_Notify::flags.armed; | ||
msg.mode = AP::vehicle()->get_mode(); | ||
msg.flying = AP_Notify::flags.flying; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be vehicle->get_likely_flying();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set as suggested
libraries/AP_DDS/AP_DDS_Client.cpp
Outdated
// Fill the new message. | ||
msg.vehicle_type = static_cast<uint8_t>(AP::fwversion().vehicle_type); | ||
msg.armed = AP_Notify::flags.armed; | ||
msg.mode = AP::vehicle()->get_mode(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should do const auto &vehicle = AP::vehicle();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set as suggested
const uint8 APM_AP_BOOTLOADER = 11; | ||
const uint8 APM_BLIMP = 12; | ||
const uint8 APM_HELI = 13; | ||
const uint8 FS_RADIO = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be a separate enum? seems odd to fix FS and APM_xxx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is auto generated, I could set the value to start from 14.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set the FS enums starting from 21
libraries/AP_DDS/AP_DDS_Client.cpp
Outdated
if ( AP_Notify::flags.failsafe_radio ) { | ||
msg.failsafe[fs_iter++] = FS_RADIO; | ||
} | ||
if ( AP_Notify::flags.failsafe_battery ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use this from AP_BattMon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set as suggested
libraries/AP_DDS/AP_DDS_Client.cpp
Outdated
if ( AP_Notify::flags.failsafe_radio ) { | ||
msg.failsafe[fs_iter++] = FS_RADIO; | ||
} | ||
if ( AP_Notify::flags.failsafe_battery ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could get the battery failsafe state from the battery library.
For RC, EKF and GCS failsafe we probably need to do what you've done here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have an opinion about it, maybe we could port that limitation to a new issue?
31ca9c3
to
25bed36
Compare
3a5f232
to
54d5b9e
Compare
54d5b9e
to
4b1a421
Compare
/ap/status
messageexternal_control
, which will be filled by or after AP_DDS: External Control enable #28429.Test
armed
flag changes.Test a failsafe:
BATT_LOW_VOLT
to 11.0SIM_BATT_VOLTAGE
to 10.6.SIM_RC_FAIL
to 1.